Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shorts #235

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix shorts #235

wants to merge 3 commits into from

Conversation

fire332
Copy link
Collaborator

@fire332 fire332 commented Dec 18, 2024

This PR is built on top of #234. That should be dealt with before this one. #234 is merged. Re-based onto main.

Fixes #210, fixes #211, fixes #219

Changes

  • Allow using TypeScript in app code by adding TypeScript support to babel. Cherry-picked into Misc dev changes #238.
  • Fix thumbnails in the shorts shelf.
  • Fix stretched/offset shorts player by changing the screensaver fix to not apply when not using the watch (full-screen player) page.
  • Disable "Remove Shorts from subscriptions" by default.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 19, 2024

Whoops. Had a brain fart when refactoring the screensaver fix.

@cremor
Copy link

cremor commented Dec 20, 2024

Thanks for working on this problem!
I've tested your changes (with a build based on #236) any I can indeed see the video of shorts again. But the video is not placed exactly in the correct area. There is a part of black space on the left side of the video area, and the right side of the video is cut off.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

@cremor can you take a picture?

@cremor
Copy link

cremor commented Dec 20, 2024

Sure. This is on an OLED C9 with WebOS 4.10.0:
Short

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

How did you take this picture? It looks more like the video is in the correct spot but the entire UI is offset

@cremor
Copy link

cremor commented Dec 20, 2024

It's just a photo I took with my phone. The short video was paused there, but that doesn't make a difference.
The thumbnail, which is visible for a moment before the short video playback starts, is not cut off. Only the actual video stream is.

@cremor
Copy link

cremor commented Dec 20, 2024

It looks more like the video is in the correct spot but the entire UI is offset

You are right. The white border isn't in the center of the screen. The video itself seems to be in the center.
But what could cause this? I never had this problem before the shorts video stopped being shown a few weeks/months back.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

Do you know how to use the web inspector tool? Can you check the computed styles of the top-most element that's offset incorrectly?

@cremor
Copy link

cremor commented Dec 20, 2024

No, but I think I could figure it out with a few hints. What software would I use to do that? I've only used WebOS Device Manager to install the app on the TV.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 20, 2024

You need node.js 22 LTS installed. Then download a copy of this repository and open the folder in terminal. Then run:

npm install
ares-inspect -a youtube.leanback.v4

@cremor
Copy link

cremor commented Dec 21, 2024

It was a bit more complicated than that 😅 I had to install the WebOS CLI tools and setup the device connection for my TV with it. Only then I could use the ares-inspect command.

But even then, how do I continue? I opened the shown "Application Debugging" URL in a browser and it showed YouTube under "Inspectable WebContents". But after clicking it I only get an empty page and the browser dev tools only show an empty body tag.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 21, 2024

Do you have the app launched on ur TV? If you don't, it's not going to work well.

@cremor
Copy link

cremor commented Dec 21, 2024

Yes, the app was running. I even had a short opened (in paused state).

@fire332
Copy link
Collaborator Author

fire332 commented Dec 21, 2024

It might be because you're using the CLI tools provided by LG. Try uninstalling it and use the CLI tools that are included by this project. Your terminal must be opened to the project directory and you need to prefix ares commands with npx for it to work.

@cremor
Copy link

cremor commented Dec 22, 2024

Same result.
But I noticed an error in the browser dev tools console: inspector.js:4031 Uncaught TypeError: document.registerElement is not a function

Seems like this is a deprecated function which is not supported in modern browsers any more: https://webostv.developer.lge.com/faq/2020-05-20-debugging-web-inspector-not-working-chrome-v80
I assume my TV (WebOS version) is too old. But TBH I don't really want to install that IDE or an old browser...

@fire332
Copy link
Collaborator Author

fire332 commented Dec 22, 2024

You can always try a portable version.

https://google-chrome-portable.en.uptodown.com/windows/download/2171677

@cremor
Copy link

cremor commented Dec 22, 2024

That "portable installer" doesn't work. It seems like that portable Chrome version isn't available any more (the error message contains a 404).

@cremor
Copy link

cremor commented Dec 22, 2024

Ok, I found a working version at https://portapps.io/app/ungoogled-chromium-portable/#download
Now I can see the HTML in the inspector.

This seems to be the top element of the border around the short video:
grafik

And here are its computed styles:
grafik

The computed styles of the video element might also be interesting:
grafik

@fire332
Copy link
Collaborator Author

fire332 commented Dec 22, 2024

Can you take a pic of your TV just like last time again but with the edges of your TV clearly visible?

@cremor
Copy link

cremor commented Dec 23, 2024

Sure:
Screen

@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Your html5-video-container dimensions and position matches up with mines so it's definitely the rest of the UI that's offset.

Can you show me the computed styles for the ytlr-player-focus-ring? You can find it easily by entering document.querySelector("ytlr-player-focus-ring") into the console, then right-click the result -> reveal in elements panel.

@cremor
Copy link

cremor commented Dec 23, 2024

Computed styles:
grafik

HTML context:
grafik

@fire332 fire332 marked this pull request as ready for review December 23, 2024 11:44
@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Show me the computed styles for the <ytlr-shorts-page> element right above it in your screenshot.

@fire332 fire332 added this to the v0.3.5 milestone Dec 23, 2024
@cremor
Copy link

cremor commented Dec 23, 2024

This is the last thing I can test with my TV until the holidays are over because I won't be home.

That element seems to cover the whole screen. But here are the computed styles anyway:
grafik

@fire332
Copy link
Collaborator Author

fire332 commented Dec 23, 2024

Your computed styles match up exactly with mines so the offset theoretically shouldn't be possible. Lemme know when you get back and feel like jumping on this problem again.

I appreciate your help. Happy holidays.

@victorrrz
Copy link

I tested this on a LG C1 (using webOS 6.4.0) and now shorts are working fine, with the previous version (v0.3.5-rc1) shorts looked bad, the video was out of frame and stretched horizontally.


/**
* Check to see if identical before assignment as some webOS versions will trigger a mutation
* mutation event even if the assignment effectively does nothing, leading to an infinite loop.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutation mutation double

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@Gagac44
Copy link

Gagac44 commented Dec 25, 2024

I also have an LG OLED C9 with WebOS 4.9.17.
In my case, the video is completely dark and you can only hear the sound. When I click OK on a short, the thumbnail appears for approx. 1 second and as soon as the video starts, the picture goes black and you can only hear the sound.

In addition, the thumbnails of the shorts in the menu or in the search are always black.

Merry Christmas! :)

@cremor
Copy link

cremor commented Dec 25, 2024

@Gagac44 I assume you are using the normal release build? This is a pull request discussion. Please only comment here if you actually tested the changes from this PR (by building the app yourself).

@fire332 fire332 modified the milestones: v0.3.5, future Dec 27, 2024
@cremor
Copy link

cremor commented Dec 28, 2024

@fire332 I'm back home and just updated to the latest build from your fix-shorts branch. I can continue testing/checking styles if you want. (The short video is still offset in the latest build.)

@fire332
Copy link
Collaborator Author

fire332 commented Dec 29, 2024

Welcome back. Can you enter this into the console and check if the borders are symmetrical?

document.head.appendChild((function(){var x=document.createElement("style");x.innerHTML="* { border: green solid 1px; }";return x;})())

@cremor
Copy link

cremor commented Dec 29, 2024

That is very weird. That command not only shows borders around all elements, but it also moves the shorts video element so that it is placed correctly. The moment I execute the command the shorts video is moved a bit to the left. I've tested this twice to confirm it.

It also looks like executing the command brings the video element in the foreground, because it then overlaps the grey border at the top and bottom.

Before executing the command:
before

After:
after

@useman
Copy link

useman commented Dec 29, 2024

lg g1 webos 03.41.05 works fine with this update

@fire332
Copy link
Collaborator Author

fire332 commented Dec 29, 2024

Is it the video that shifts back or the UI?

@cremor
Copy link

cremor commented Dec 29, 2024

The video.

@fire332
Copy link
Collaborator Author

fire332 commented Dec 30, 2024

@cremor can you run the the same command in console but replace the * with * > video?

@cremor
Copy link

cremor commented Dec 30, 2024

This also moves the video frame, but this time it's not in the front (so the rounded borders still look fine).
The green border from your command only shows up on the right side of the video frame.

With video frame border selected (you have to zoom in to see the green border on the right side):
focus

With something else selected:
no focus

@fire332
Copy link
Collaborator Author

fire332 commented Dec 30, 2024

What happens if you set the border to 0px width instead of 1px?

@cremor
Copy link

cremor commented Dec 30, 2024

Nothing happens when I do this.
And I can even move the video frame back to the wrong position when I execute it with 0px (after executing it with 1px).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants